Skip to content

Cache field metadata during checking and planning#913

Merged
snazy merged 1 commit into
projectnessie:mainfrom
snazy:field-meta-cache
Jul 17, 2026
Merged

Cache field metadata during checking and planning#913
snazy merged 1 commit into
projectnessie:mainfrom
snazy:field-meta-cache

Conversation

@snazy

@snazy snazy commented Jul 17, 2026

Copy link
Copy Markdown
Member

Avoid repeated provider field metadata lookups within a checker or planner instance. Reuse checked select field metadata when constructing field qualifiers instead of immediately querying the provider again.

Avoid repeated provider field metadata lookups within a checker or planner instance. Reuse checked select field metadata when constructing field qualifiers instead of immediately querying the provider again.
@snazy
snazy requested a review from XN137 July 17, 2026 07:54
fieldType != null
? new AttributeFactory.FieldQualifier(
expr.getId(), sel.getField(), fieldType, adapter)
: attrFactory.newQualifier(opType, expr.getId(), sel.getField());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i assume this change avoids a redundant provider.findFieldType call in AttrFactory.newQualifier ?
is the field lookup that expensive to be worth kind of breaking the factory abstraction?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit of repeated work, yea.

Type opType = typeMap.get(sel.getOperand().getId());
if (opType != null && !opType.getMessageType().isEmpty()) {
FieldType ft = provider.findFieldType(opType.getMessageType(), sel.getField());
FieldType ft = findFieldType(opType.getMessageType(), sel.getField());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i assume its not worth caching negative lookup results?
(i have no idea how frequently this gets called and how expensive provider.findFieldType is)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really, no

@snazy
snazy merged commit 5ce4230 into projectnessie:main Jul 17, 2026
3 checks passed
@snazy
snazy deleted the field-meta-cache branch July 17, 2026 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants